You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The reason will be displayed to describe this comment to others. Learn more.
Verified review notes
The updated guide correctly distinguishes options-capable native capture from legacy access-only capture and explains why guarded WPR excludes brokered WFP network records. The two inline comments identify a newly incorrect .NET API reference and the undocumented PSEC capture-mode capability exception. This is a comment-only review.
Finding outside the diff
Low (documentation consistency) — Update the README's native/fallback shorthand.README.md:265 — Attribution:newly_exposed_by_change. The README is byte-identical between this PR's base and head, so the earlier wording is not presented as code changed here. This PR's new host-selection guide recognizes compatible legacy-start PSEC hosts as native access-only captures; the README still describes 'complete PSEC/V2' native hosts versus 'older' guarded-WPR tiers, leaving readers of the two linked documents with conflicting classifications. Fix: Align the README's audit-mode summary with the new compatible-export rule and distinguish legacy native capture from guarded WPR.
Verified pre-existing — not attributed to this PR
README.md and docs/process-container/networking.md are both byte-identical from base to head. Their old wording is relevant only insofar as the new guide introduces a conflicting host-selection description or newly documents capture behavior without qualifying the existing capability matrix. No unrelated pre-existing issue is attributed to this documentation PR.
Gudge (@MGudgin) The documentation findings are fixed in 92693ee: the V1 .NET type names are restored, and both the denial-capture guide and ProcessContainer networking matrix explain the narrowly scoped native PSEC capture capability exception. The docs are restacked on the implementation fix. Please re-review the updated tip.
Document the source-domain and WFP provider/sublayer identity required before interpreting Tessera reasons. Clarify that both option-aware and legacy access-only native capture emit verbose version 5, while guarded WPR emits version 4.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16300264-31dd-4214-a056-f01395e3179e
Clarify schema-failure behavior versus actionable network denials
docs/logging-access-denied.md:269
This newly documented actionable network-denial behavior contradicts the existing schema-failure paragraph at lines 517–520, which still says “network decisions are not denials.” The implementation records network-provider schema failures diagnostically without setting deniedResourcesTruncated; update that paragraph to describe this behavior without denying the actionable contract introduced here.
Document all networkDecisionReason field values
docs/logging-access-denied.md:286
The table does not document the serialized networkDecisionReason contract promised by the PR: only directDefaultDeny appears later in an example, leaving consumers unable to discover appIsolationMissingCapability, authoredExplicitDeny, allowExclusion, and proxyContainment. Add the exact field values for every row.
List every serialized networkDecisionReason value. Clarify network-provider schema failures do not set deniedResourcesTruncated, without contradicting actionable decoded network denials or the guarded-WPR exclusion.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16300264-31dd-4214-a056-f01395e3179e
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The documentation does not explicitly state the promised internal-only status of the Tessera name.
0 open findings
Previously missed (1)
In code that hasn't changed since last review
Clarify Tessera is internal-only and absent from public JSON
docs/logging-access-denied.md:269
The PR description promises to clarify that Tessera is only an internal component name and is not exposed in public JSON, but this section introduces Tessera as a source domain without stating that boundary. This can make readers infer a caller-facing source: "Tessera" field even though the public record below has no such field. Please make the internal-only status explicit here.
Addressed both previously missed findings from the latest Copilot review in f04896c. The source/reason table now lists all five serialized networkDecisionReason values. The schema-failure paragraph now states that network-provider schema failures are diagnostic-only and do not set deniedResourcesTruncated, while successfully decoded network events may produce actionable denials. The adjacent stale unsupportedEventSchema wording was also corrected to match native decoding and guarded-WPR scoping.
Explicitly distinguish the internal component name from the public denial record and verbose JSON provider/reason vocabulary.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16300264-31dd-4214-a056-f01395e3179e
Addressed the latest summary-only Copilot finding in 0ed5164. The network-denial section now explicitly identifies Tessera as an internal Windows component name, states that it is not exposed in public JSON, and distinguishes the source-free actionable record from the learningModeNetworkDecision provider and typed networkDecisionReason vocabulary in verbose logging.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The updated behavior conflicts with existing proxy and ETL-retention guidance in docs/schema.md.
0 open findings
Previously missed (2)
In code that hasn't changed since last review
Correct schema guidance for native capture with network proxy
docs/logging-access-denied.md:258
This proxy-mode behavior conflicts with docs/schema.md:259-261, which still says native capture cannot be combined with runtimeConfig.networkProxy. The implementation explicitly accepts capture with a runtime proxy (base_container_runner.rs:3836-3845) while preserving the proxy capability posture (base_container_runner.rs:3128-3153). Please update that schema guidance so callers are not told this supported combination will fall back or fail.
Update schema guidance for guarded-WPR retention fallback
docs/logging-access-denied.md:582
The canonical configuration guide still states that the guarded-WPR fallback rejects retention (docs/schema.md:255-256), contradicting this new guarantee. The product PlmGuardedCaptureFactory enables trace transfer (src/mxc-sdk/src/core/mxc_engine/guarded_capture.rs:203-210), so please update the schema guidance to document the supported fallback behavior consistently.
Addressed both previously missed schema-guide findings in 7df6c67. docs/schema.md now documents native PSEC capture with runtimeConfig.networkProxy while preserving the proxy capability posture and leastPrivilege/other-proxy fallback. It also documents guarded-WPR retention of a workload-process-filtered ETL, never the host-wide source, and replaces stale PSEC/V2-only guidance.
This complete-looking artifact cannot be emitted as shown. An accepted NetworkDecisionV1 must contain all 24 schema properties, and the producer sanitizes the full property list; only timestamp-like entries such as OriginalTimestamp are dropped, so the resulting properties array contains many more entries than ApplicationId and FilterId. Please either show the complete retained array or explicitly mark this as an abridged, non-literal excerpt so consumers do not use it as the expected producer output.
Clarify that the shortened properties array is illustrative rather than literal producer output or a fixture, and that accepted events retain sanitized schema properties subject to documented omissions and bounds.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 16300264-31dd-4214-a056-f01395e3179e
Addressed the latest summary-only Copilot finding in cb48248. The version-5 JSON example is now explicitly labeled an abridged, non-literal illustration, not complete producer output or an expected-output fixture. The note explains that only two properties are shown and that accepted NetworkDecisionV1 events contain all 24 schema properties, retained after the documented sanitization, omissions, and bounds.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
The public IPv4 and IPv6 ICMP resource representations remain ambiguous.
0 open findings
Previously missed (1)
In code that hasn't changed since last review
Document distinct ICMP and ICMPv6 endpoint representations
docs/logging-access-denied.md:367
The ICMP endpoint contract is still ambiguous here because the two public representations intentionally differ. An actionable IPv4 denial uses resource: "icmp://…", an IPv6 denial uses resource: "icmpv6://[…]", but the version-5 networkEndpoint.protocol and the authored policy both use "icmp" for either family (network_extractors.rs:400-433, 751-775). Document both resource schemes so consumers do not copy icmpv6 into the policy protocol or parse IPv6 records as icmp://.
Addressed the latest Copilot summary finding in 46273a1. The guide now explicitly distinguishes portless IPv4 icmp://203.0.113.10 and IPv6 icmpv6://[2001:db8::1] resources. Both structured networkEndpoint.protocol and the authored policy use icmp, with no port field; the text warns not to copy icmpv6 into policy protocol.
The reason will be displayed to describe this comment to others. Learn more.
🟢 Approval recommended
The documentation matches the implementation, and no unresolved issues remain.
0 open findings
🧠 Review effort: Balanced
This branch has not been deployed
No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📖 Description
Documents WFP denial capture, runtime compatibility probing, and the caller-facing network denial contract.
filterIdrepresentation.This is PR 4 of 4 in the WFP Learning Mode stack. Its base is the integration-layer branch.
🔗 References
Related to #1286.
Depends on the preceding integration PR in this stack.
🔍 Validation
git diff --checkacross the complete stack — passed.✅ Checklist
Cargo.lock, thedependency-feed-checkcheck passes (not applicable)📋 Issue Type
🧱 Stack
Review and merge in this order.
Microsoft Reviewers: Open in CodeFlow